Skip to content

Document a known limitation regarding affected row count #202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

JanJakes
Copy link
Collaborator

@JanJakes JanJakes commented Jul 2, 2025

This PR adds a file to document known SQLite driver limitations, and it documents the first one.

@JanJakes JanJakes requested a review from adamziel July 2, 2025 10:40
@adamziel
Copy link
Collaborator

adamziel commented Jul 2, 2025

Would a SELECT query within the same, isolated transaction give us the number of rows matched but not updated?

@JanJakes
Copy link
Collaborator Author

JanJakes commented Jul 2, 2025

@adamziel I think so, yes, but in SQLite, that seems to equal the number from SELECT CHANGES() and the row cound from PDO APIs.

@adamziel
Copy link
Collaborator

adamziel commented Jul 2, 2025

We'd need to add a condition such as SELECT * FROM target_table WHERE ($original_update_conditions) to get the number of matching rows and a condition such as SELECT * FROM target_table WHERE ($original_update_conditions) AND NOT (field1 = :updated_value1 OR field2 = :updated_value2) to get the number of changed rows. It is a fair limitation to have, but it also seems like a tractible problem.

@JanJakes
Copy link
Collaborator Author

JanJakes commented Jul 2, 2025

@adamziel Ah, right, I see what you mean now. This approach could likely be applied directly to the UPDATE by rewriting it to directly exclude the rows where the existing values are equal to the new ones:

UPDATE ... SET col1 = val1, col2 = val2 WHERE ... AND (col1 != val1 OR col2 != val2)

Then the number of affected rows should be correct, but maybe the caveat is that in edge cases like with triggers, this could get tricky. That said, we don't support MySQL triggers and can probably never fully support them.

@adamziel
Copy link
Collaborator

adamziel commented Jul 2, 2025

Really good idea to fold this directly into the update query.

It's fine to disregard triggers here. They're not a big priority at this point. We could leave a comment to document what needs to be done if we ever get to support triggers.

@JanJakes
Copy link
Collaborator Author

JanJakes commented Jul 3, 2025

@adamziel In that case, I think I can close this PR and create an issue to implement this into the UPDATE query instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants